-
-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Add unnecessary_hook_widget
lint rule and quick fix
#271
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @akaboshinit, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
This pull request introduces a new lint rule, unnecessary_hook_widget
, to the nilts_flutter_hooks
package. This rule identifies instances where HookWidget
is used without utilizing any hooks, suggesting a replacement with StatelessWidget
for better performance and code clarity. A quick fix is also provided to automatically replace HookWidget
with StatelessWidget
when appropriate. The PR also includes updates to the README to document the new lint rule, and adds a test case for the new lint rule.
Highlights
- New Lint Rule: unnecessary_hook_widget: Adds a lint rule that flags
HookWidget
usages where no hooks are present, suggestingStatelessWidget
as an alternative. - Quick Fix Implementation: Provides a quick fix to automatically replace unnecessary
HookWidget
usages withStatelessWidget
. - Documentation Update: Updates the README file to include details and examples for the new
unnecessary_hook_widget
lint rule. - Test Case: Adds a test case to verify the functionality of the new lint rule.
Changelog
Click here to see the changelog
- packages/nilts/README.md
- Added documentation for the
unnecessary_hook_widget
lint rule, including a description, target SDK, rule type, maturity level, and examples of good and bad code.
- Added documentation for the
- packages/nilts_flutter_hooks/.gitignore
- Added
.gitignore
file.
- Added
- packages/nilts_flutter_hooks/CHANGELOG.md
- Added
CHANGELOG.md
file.
- Added
- packages/nilts_flutter_hooks/README.md
- Added
README.md
file.
- Added
- packages/nilts_flutter_hooks/analysis_options.yaml
- Added
analysis_options.yaml
file.
- Added
- packages/nilts_flutter_hooks/example/example.md
- Added
example.md
file.
- Added
- packages/nilts_flutter_hooks/lib/nilts_flutter_hooks.dart
- Introduced the
_NiltsFlutterHooksLint
class to define and register theUnnecessaryHookWidget
lint rule.
- Introduced the
- packages/nilts_flutter_hooks/lib/src/change_priority.dart
- Added
ChangePriority
class to define the priority of the quick fix for replacingHookWidget
withStatelessWidget
.
- Added
- packages/nilts_flutter_hooks/lib/src/lints/unnecessary_hook_widget.dart
- Implemented the
UnnecessaryHookWidget
lint rule, including the logic to detect unnecessaryHookWidget
usages and provide a quick fix to replace them withStatelessWidget
.
- Implemented the
- packages/nilts_flutter_hooks/pubspec.yaml
- Created
pubspec.yaml
file.
- Created
- packages/nilts_test/pubspec.yaml
- Added
flutter_hooks
as a dependency. - Added
nilts_flutter_hooks
as a dependency.
- Added
- packages/nilts_test/test/hooks_lints/unnecessary_hook_widget.dart
- Created a test file for the
unnecessary_hook_widget
lint rule, including test cases for both failing and passing scenarios.
- Created a test file for the
- release-please-config.json
- Added configuration for
nilts_flutter_hooks
package.
- Added configuration for
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
A HookWidget stands so tall,
But no hooks it uses at all.
The linter cries, 'Oh no, you see,
A StatelessWidget it should be!'
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
The pull request introduces a new lint rule, unnecessary_hook_widget
, which identifies opportunities to replace HookWidget
with StatelessWidget
when no hooks are used. The rule includes a quick fix. Overall, the implementation seems well-structured and includes relevant documentation and tests. However, there are a few areas that could be improved for clarity and consistency.
Merge Readiness
The pull request is almost ready for merging. I recommend addressing the comments to improve code clarity and consistency. I am unable to directly approve this pull request, and recommend that others review and approve this code before merging.
unnecessary_hook_widget
lint rule and quick fix
@@ -725,6 +726,51 @@ See also: | |||
|
|||
</details> | |||
|
|||
#### unnecessary_hook_widget |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can write details after your PR merged.
Write minimal documentations please.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll configure this before release.
You need not to add this, thanks.
@@ -0,0 +1,34 @@ | |||
name: nilts_flutter_hooks | |||
version: 0.19.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
) { | ||
context.registry.addClassDeclaration((node) { | ||
final superclass = node.extendsClause?.superclass; | ||
if (superclass == null || superclass.toString() != 'HookWidget') return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add package name check.
You can define getter in nilts_core.
https://github.com/dassssshers/nilts/blob/main/packages/nilts_core/lib/src/utils/library_element_ext.dart
|
||
@override | ||
void visitMethodInvocation(MethodInvocation node) { | ||
if (node.methodName.name.startsWith('use')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
} | ||
|
||
@override | ||
void visitFunctionExpressionInvocation(FunctionExpressionInvocation node) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -0,0 +1,34 @@ | |||
name: nilts_flutter_hooks | |||
version: 0.19.0 | |||
description: nilts is lint rules, quick fixes and assists for Dart and Flutter projects that helps you enforce best practices, and avoid errors. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
description: nilts is lint rules, quick fixes and assists for Dart and Flutter projects that helps you enforce best practices, and avoid errors. | |
description: nilts_flutter_hooks is lint rules, quick fixes and assists for Dart and Flutter projects using flutter_hooks that helps you enforce best practices, and avoid errors. |
Overview
add rule unnecessary_hook_widget
package split nilts_flutter_hooks
Fixes #
Feature type
Checklist
melos test
to run tests.